SIG-5341 v4 prep before new major #161
Conversation
ATimmeh33
force-pushed
the
SIG-5341-v4-final
branch
from
360edab to
73cf31e
Compare
There was a problem hiding this comment.
Pull request overview
This PR prepares the SignhostAPIClient for a v5 major release by adding new API features, improving error handling, and establishing integration tests. The changes include new verification types (Onfido, OIDC, eHerkenning, CSC), enhanced data models with additional properties, and a new ResponseBody property for exception handling.
Key changes:
- Added new verification types (OnfidoVerification, OidcVerification, EherkenningVerification, CscVerification) to support additional authentication methods
- Enhanced exception handling by adding ResponseBody property to SignhostRestApiClientException and refactoring error handling logic
- Created integration test infrastructure with user secrets configuration to test against live API
Reviewed changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SignhostRestApiClientException.cs | Added ResponseBody property to expose API error details |
| SignhostException.cs | Added TODO comment for v5 removal |
| HttpResponseMessageErrorHandlingExtensions.cs | Refactored exception throwing to use switch-and-assign pattern; added ResponseBody population |
| BadAuthorizationException.cs | Added TODO comment for v5 usage |
| SurfnetVerification.cs | Added Uid and Attributes properties |
| Signer.cs | Added timestamp and URL properties for signer lifecycle tracking |
| Receiver.cs | Added Id and timestamp properties |
| PhoneNumberVerification.cs | Added SecureDownload property |
| OnfidoVerification.cs | New verification type for Onfido identity verification |
| OidcVerification.cs | New verification type for OpenID Connect |
| IVerification.cs | Added TODO comment for v5 interface split |
| Field.cs | Added TODO comments for type improvements in v5 |
| EherkenningVerification.cs | New verification type for eHerkenning authentication |
| DigidVerification.cs | Added SecureDownload property |
| CscVerification.cs | New verification type for Cloud Signature Consortium |
| ActivityType.cs | Added TODO comment for cleanup in v5 |
| Activity.cs | Added ActivityValue property with JSON mapping |
| ApiResponse.cs | Added ResponseBody to GoneException with async workaround |
| SignhostAPIClient.sln | Added integration tests project reference |
| TransactionTests.cs | New comprehensive integration test for transaction lifecycle |
| TestConfiguration.cs | Configuration loader for integration test credentials |
| SignhostAPIClient.IntegrationTests.csproj | Project file for integration tests |
| README.md | Documentation for integration test setup |
| publish_nuget_package.yml | Added comment about integration tests exclusion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| break; | ||
|
|
||
| case HttpStatusCode.PaymentRequired | ||
| when errorType == "https://api.signhost.com/problem/subscription/out-of-credits": |
There was a problem hiding this comment.
The hardcoded URL string should use the OutOfCreditsApiProblemType constant defined on line 15 instead of duplicating the value.
| when errorType == "https://api.signhost.com/problem/subscription/out-of-credits": | |
| when errorType == OutOfCreditsApiProblemType: |
src/SignhostAPIClient/Rest/ErrorHandling/HttpResponseMessageErrorHandlingExtensions.cs
Show resolved
Hide resolved
ATimmeh33
force-pushed
the
SIG-5341-v4-final
branch
from
73cf31e to
6f004ef
Compare
| /// <summary> | ||
| /// Gets or sets the certificate issuer. | ||
| /// </summary> | ||
| public string Issuer { get; set; } |
There was a problem hiding this comment.
Where did you get this def, for api the Provider only exposed and not Issuer, Subject, Thumbprint, AdditionalUserData they come from the settings as far as I remember.
There was a problem hiding this comment.
There was a problem hiding this comment.
We do expose them already, these are all returned in the postback/GETs after successful CSC verification
There was a problem hiding this comment.
I assume that CscVerification class is only for creation of the transaction and not about postabcks or other gets, otherwise like OIDCVerification also would expose something else. Sync with Berend too on this, seems strange if this class is only for the TranactionCreation purpose by Client to also include other attributes of the response.
BerendJonkman
left a comment
BerendJonkman
left a comment
There was a problem hiding this comment.
Approved tech & style - 6f004ef29150bfab2e7e96ea8393caf06ef9b51b
This pull request introduces a new integration test project for the Signhost API client, adds comprehensive integration tests that require live credentials, and extends support for new verification types in the data model. It also includes minor improvements and clarifications in the codebase and CI/CD configuration.
Integration Testing Infrastructure:
SignhostAPIClient.IntegrationTestswith configuration for .NET 8, user secrets, and necessary dependencies. This project is now included in the solution file and is excluded from packaging and CI/CD runs. [1] [2] [3] [4] [5]TransactionTests.cs) that exercises transaction creation, file upload, and transaction state verification, ensuring real-world API functionality.README.mdwith clear instructions for configuring and running integration tests safely with user secrets.TestConfigurationclass that loads credentials only from user secrets to prevent accidental credential leaks.Verification and Authentication Model Extensions:
CscVerification,EherkenningVerification,OidcVerification, andOnfidoVerification, expanding the range of supported identity and signature verification methods. [1] [2] [3] [4]PhoneNumberVerificationandDigidVerificationto include aSecureDownloadproperty. [1] [2]Data Model and Serialization Improvements:
Activityclass with a newActivityValueproperty and JSON serialization attributes. [1] [2]ActivityTypeandFieldclasses for future refactoring. [1] [2]Error Handling and Miscellaneous: